-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[web] [SIMS #254]Used symbols, updated components to use symbols #274
Conversation
}, | ||
setup(props: any) { | ||
const router = useRouter(); | ||
console.log("props"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove console logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
LOGIN: Symbol(), | ||
STUDENTPROFILE: Symbol(), | ||
STUDENTPROFILEEDIT: Symbol(), | ||
FINANCIALAIDAPPLICATION: Symbol(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider using "_" to provide an easier reading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -0,0 +1,17 @@ | |||
export const routeConstants = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend segregation between Students and Institutions routes. IMHO it would be easier to read and maintain.
It would be something like "routes.student.FINANCIAL_AID_APPLICATION" and "routes.institution.DASHBOARD".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
@@ -2,20 +2,27 @@ | |||
<div class="p-component"> | |||
<div class="p-card p-m-4"> | |||
<Section title="Confirm Submission Page..." /> | |||
<FooterNavigator previous="financial-info" /> | |||
<FooterNavigator v-bind:previous="financialInfo" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please condider use only the shortcut for v-bind (:previous="financialInfo") instead of (v-bind:previous="financialInfo").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just like using v-bind because it's more explicit. A little old school in this. Hope this is ok
sources/packages/web/src/views/student/financial-aid-application/ConfirmSubmission.vue
Outdated
Show resolved
Hide resolved
@@ -93,13 +93,17 @@ | |||
</ContentGroup> | |||
</Section> | |||
</ContentGroup> | |||
<FooterNavigator previous="select-program" next="confirm-submission" /> | |||
<FooterNavigator | |||
v-bind:previous="selectProgram" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider using the shortcut for v-bind (:previous) instead (v-bind:previous).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just like using v-bind because it's more explicit. A little old school in this. Hope this is ok
sources/packages/web/src/views/student/financial-aid-application/SelectProgram.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work having the names of the routes organized 👍
If we need access to these constantly from the template, we can look into a way to declare the route constants globally.
@@ -4,7 +4,8 @@ module.exports = { | |||
"^.+\\.vue$": "vue-jest", | |||
}, | |||
globals: { | |||
"ts-jest": { | |||
"ts-jest": { | |||
babelConfig: 'babel.config.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please adjust the double quotes to be aligned with the rest of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work with the constants 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend name of the route constant should be start with Capital letter.
@@ -0,0 +1,17 @@ | |||
export const routeConstants = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Const object name should start with Capital letter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
**As a part of this PR, release v2.2.0 is merged to v2.3.0** --------- Co-authored-by: Bidyashish <[email protected]> Co-authored-by: Andrew Boni Signori <[email protected]>
**As a part of this PR, release v2.3.0 is merged to main. This PR contains the changes that were directly made into the release branch 2.2 and were updated to the release branch 2.3 when the release branch 2.2 was merged to 2.3** Co-authored-by: Bidyashish <[email protected]> Co-authored-by: Andrew Boni Signori <[email protected]>
Release 2.3 from intermediate branch. #4223 [- Student Bridge Match - PPD Status Not Importing ](ffb5aa5) --------- Co-authored-by: Shashank Shekhar <[email protected]> Co-authored-by: Andrew Boni Signori <[email protected]> Co-authored-by: Dheepak Ramanathan <[email protected]>
No description provided.